Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chronos handler to support chronos-bolt-* models #223

Open
wants to merge 40 commits into
base: new_model_integrations
Choose a base branch
from

Conversation

gganapavarapu
Copy link
Collaborator

This PR is to enable chronos-bolt-tiny model tsfm inference service.

wgifford and others added 30 commits November 26, 2024 13:34
Address some issues in forecasting pipeline
@wgifford
Copy link
Collaborator

@gganapavarapu Looks like it is failing because we are passing ID columns and this is not yet supported for Chronos?

@ssiegel95
Copy link
Collaborator

Something is up with the freq parameter too.

@gganapavarapu
Copy link
Collaborator Author

@gganapavarapu Looks like it is failing because we are passing ID columns and this is not yet supported for Chronos?

Correct, ID columns is not supported. But, I was using only target columns in chronos handler. So, this should not be problem. [10] not found in axis is because the input data length for chronos is 1 as minimum_context_length is set to 1, and we are trying to drop this row in the input data in our tests.

@gganapavarapu
Copy link
Collaborator Author

TimeSeriesProcessor is not able to derive frequency as input data length is 1 in chronos lib tests. Chronos service tests are passing. So, either we need to handle this in TSFM service components or increase minimum_context_length in chronos tsfm_config.json. Our tests are expecting minimum of length 11.

@gganapavarapu
Copy link
Collaborator Author

Running few tests to evaluate the options.

@ssiegel95
Copy link
Collaborator

Regarding increasing the minimum context length, that seems reasonable to me but @wgifford are there common use cases out there where someone would expect to get a forecast with a very small context like 1? I can see this for more traditional approaches like Arima but for DL-based models it seems like a degenerate case to me.

@gganapavarapu gganapavarapu marked this pull request as draft December 10, 2024 17:23
@gganapavarapu
Copy link
Collaborator Author

converting this PR to draft until all units are passed.

@gganapavarapu
Copy link
Collaborator Author

Merged main branch into chrono_bolt branch. This requires review of all the files from merge.

@gganapavarapu gganapavarapu marked this pull request as ready for review December 12, 2024 19:11
@gganapavarapu
Copy link
Collaborator Author

gganapavarapu commented Dec 12, 2024

Regarding increasing the minimum context length, that seems reasonable to me but @wgifford are there common use cases out there where someone would expect to get a forecast with a very small context like 1? I can see this for more traditional approaches like Arima but for DL-based models it seems like a degenerate case to me.

minimum context length is increased to 16. Tests are passing now without changes. Thank you @wgifford .

@gganapavarapu
Copy link
Collaborator Author

This PR supports ID columns and model specific parameters for chronos models.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants